This function could easily be made a method of the Pin class (no change to the C code would be needed). It seems sensible to make it a method of Pin because the function does not work on any other objects.

First of all, I appreciate following my idea of making it a standalone functional algorithm along the line of functions like len(), sorted(), etc. And I don't think that what you say above is true. Or rather, it can be true only because we didn't implement Pin API completely and in its full power.

So, how about we make this patch work on unix port, and be able to time any Pin class defined in Python?

Thanks for this!

It first waits while the pin input is different to the given level

Any thoughts on my suggestion in #2052 to support not first waiting for the input pin to be different from the current level? It'd be trivial to add and this way pulses that have already started could be timed too.

Any thoughts on my suggestion in #2052 to support not first waiting for the input pin to be different from the current level? It'd be trivial to add and this way pulses that have already started could be timed too.

+1. Consider you DHT considerations: storing read value into an array should take less than 50us. What if it takes 52us ans sensor already started a new pulse? As implemented now, everything will break, With an option suggested by @roger- , it will work.

Yup, the DHT is exactly what I had in mind!

One problem is arg list bloat. Can replace "level" to be bitmask of options instead.

Any thoughts on my suggestion in #2052 to support not first waiting for the input pin to be different from the current level? It'd be trivial to add and this way pulses that have already started could be timed too.

It already does this. In fact, this is the only behaviour it has: "It first waits while the pin input is different to the given level, then times the duration that the pin is equal to the given level". For example, if the argument level=1 then it waits while the pin value is 0 before starting timing, but if the pin is already 1 then it doesn't wait.

One problem is arg list bloat. Can replace "level" to be bitmask of options instead.

Yeah, how about a trigger mask:

TRIG_VALUE = 0          # trigger on pin value, like current code
TRIG_EDGE  = 2          # start timing on given edge, rising or falling 

machine.time_pulse_us(pin, 1 | TRIG_EDGE) # time a rising -> falling pulse 

It already does this. In fact, this is the only behaviour it has

Ah right, didn't look closely at your code.

I can imagine cases where that behavior might not be desired though, and the user might prefer to get a timeout error if the pulse is missing both edges.

So, how about we make this patch work on unix port, and be able to time any Pin class defined in Python?

import umachine
import utime


class MyPin(umachine.PinBase):

    def __init__(self):
        print("__init__")
        self.start = None

    def value(self):
#        print("foo")
        if self.start is None:
            self.start = utime.ticks_us()
            return 0
        d = utime.ticks_diff(self.start, utime.ticks_us())
        print("d", d)
        if d < 2000:
            return 1
        return 0


p = MyPin()

if 0:
    print(p.value())
    print(p.value())
    print(p.value())
    utime.sleep(0.001)
    print("last",p.value())
else:
    print(umachine.time_pulse_us(p, 1))

One of the better results on a overloaded Linux system:

__init__
d 2
d 31
d 51
d 70
d 91
d 112
d 132
d 153
d 175
d 199
d 220
d 241
d 263
d 284
d 305
d 327
d 349
d 373
d 395
d 416
d 438
d 458
d 480
d 502
d 524
d 545
d 569
d 590
d 611
d 633
d 671
d 694
d 715
d 738
d 761
d 783
d 804
d 826
d 846
d 868
d 890
d 911
d 935
d 957
d 978
d 1000
d 1020
d 1043
d 1065
d 1088
d 1109
d 1131
d 1152
d 1173
d 1195
d 1217
d 1240
d 1261
d 1282
d 1304
d 1325
d 1347
d 1368
d 1390
d 1413
d 1434
d 1456
d 1477
d 1497
d 1518
d 1539
d 1560
d 1582
d 1606
d 1628
d 1649
d 1670
d 1691
d 1713
d 1735
d 1756
d 1779
d 1800
d 1822
d 1843
d 1864
d 1885
d 1907
d 1929
d 1952
d 1973
d 1994
d 2016
2005

@dpgeorge : Consequently please move the implementation to extmod/machine_pulse.c

Consequently please move the implementation to extmod/machine_pulse.c

I still don't see how time_pulse_us can be a universal function. As it's written it requires a pin object as the first argument so it may as well be a pin method (eg in PinBase's method list).

If it wants to be universal then it should call value() on the incoming "pin" object, instead of using mp_pin_hal_read. But that will be terribly inefficient. One would need to have a special case to check for a pin object, and a general case to call value().

I still don't see how time_pulse_us can be a universal function.

Well, it already is, it would take additional steps to not make it so.

As it's written it requires a pin object as the first argument so it may as well be a pin method (eg in PinBase's method list).

Arbitrary pin type class is not subclass of PinBase, so PinBase having that method is of no help to them. We also don't use such pattern anywhere else, and this thing wouldn't be the best to start with (inheritance in non-compiled language == overhead, hardware operations IN_NEED_OF low overhead). Finally, we can't do that right away anyway (inheritance is not supported for native type), so we would go a long-long way to make people add more characters to implement pin type in C then really needed, make it consume more bytes, and run for more cycles. What for?

Arbitrary pin type class is not subclass of PinBase, so PinBase having that method is of no help to them.

For ports that don't use PinBase, they would put the time_pulse_us method directly in the method table of the pin object that they define. So there's no overhead there, and it'll just work as-is.

So my proposal is:

1. Implementation and type signature of time_pulse_us stays the same.
2. For ports that have their own pin type, they just put time_pulse_us in their method table.
3. For ports that use virtual pin and PinBase, they put time_pulse_us in the PinBase table.

So there's no overhead there, and it'll just work as-is.

How is there no overhead, if it requires:

1. Knowing that this needs to be done.
2. Repetitive doing for each and every port.

How is there no overhead,

There's no computation overhead when doing the operation.

Making it a function puts overhead on the usage as the user must know where it is (which module is sensible for it?). It's easier (more intuitive) from the user point of view to have it as a method.

And while I agree that per "classical type-based OO approach", this function would indeed be a method in an abstract base class, from which every concrete class would need to inherit, "classical type-based" approach is not the only approach to OO. Languages like Python or C++ heavily use "ducktyping approach", where there's no formal requirement for inheritance from a common base class (such requirement poses multiple inheritance issues). Instead, part of their OO model is based on the concept of "protocol" (Python term). With it, anything which provides methods required by a protocol is instance of that protocol. Operations which work om protocol classes/instances are called "algorithms" in C++ terms, implemented as a standalone functions, and C++ offers whole bunch of them:
http://www.cplusplus.com/reference/algorithm/ . Examples of "algorithms" in Python were given above: len(), sorted(), reverse(), etc.

So, while time_pulse_us() could be a Pin class method, there's no should if that's the concern. We continue MicroPython library of Pin algorithms, if which there're already (not fully generalized) I2C and SPI examples.

(continuing) Quite unsurprisingly, bitbanging I2C and SPI are not methods of a Pin class, because it would be quite strange to call it as a method on one pin, and pass other as params. Also, both I2C and SPI are complex algos, so they aren't just functions, but classes.

So, it requires just a one final small step to note that a function like time_pulse_us has much more in common with algos like SPI, I2C, and bunch of other we may add, than with Pin methods like .on(), .off(), .get() (or .value() with subsumes them all). The benefit of this understanding is that Pin class stays very simple, both conceptually and implementation-wise, with obvious benefit to MicroPython's reach out. If we do it right, we will be copied by other people, when they in couple of years grasp all teh benefit of a truly layered OO model, instead of "make everything a method" one.

Making it a function puts overhead on the usage as the user must know where it is (which module is sensible for it?).

It's obvious - machine.

It's easier (more intuitive) from the user point of view to have it as a method.

Just as treating CS part of SPI bus (what it isn't) or round Pi to 3. We shouldn't look for oversimplifying solutions, but ones which enable more powerful usage. Simplicity from them comes at different level (in Pin class).

1. Knowing that this needs to be done.
2. Repetitive doing for each and every port.

1. From repetition in each Pin class, actual space overhead comes, if a port uses more than one Pin class (what should be welcomed).

Ok, your arguments are good (in particular "it requires just a one final small step to note that a function like time_pulse_us has much more in common with algos like SPI, I2C, and bunch of other we may add, than with Pin methods"). So let's go for a function in the machine module.

I guess we stick with the time_pulse_us name? To leave room for time_pulse_ms and time_pulse (in seconds) if needed?

While watching this debate occurred to me to implement also the popular functions from the world of Arduino...

• ShiftIn (https://www.arduino.cc/en/Reference/ShiftIn),
• ShiftOut (https://www.arduino.cc/en/Reference/ShiftOut).
• add another extra parameter... number of bits.

There is a number of situations where they are very useful.
What is your opinion?

So let's go for a function in the machine module.

Thanks!

I guess we stick with the time_pulse_us name? To leave room for time_pulse_ms and time_pulse (in seconds) if needed?

Yes, IMHO the name is perfectly ok, and is self-descriptive, even if we won't ever get time_pulse_ms, etc. I'm less sure about timeout arg. By default for this single function it would be in microseconds, but if we add other similar functions, such "local least surprise" may lead to inconsistency. Above you say that other funcs take timeout in ms, if you're sure that's indeed the case (for new API), then maybe it's good idea to go that way for this func too.

While watching this debate occurred to me to implement also the popular functions from the world of Arduino... shiftIn, shiftOut.

These are just SPI commands. The machine.SPI object should suffice for such cases.

I'm less sure about timeout arg. By default for this single function it would be in microseconds, but if we add other similar functions, such "local least surprise" may lead to inconsistency. Above you say that other funcs take timeout in ms, if you're sure that's indeed the case (for new API), then maybe it's good idea to go that way for this func too.

When implementing the DHT driver I needed a C version of time_pulse_us, so it's already done here: https://github.com/micropython/micropython/blob/master/drivers/dht/dht.c#L34-L48 (this will move to extmod/machine_pulse.c and be a public function at the C level, plus a small Python wrapper).

You can see in the DHT driver that it uses timeouts of the order 100us. This is because if the device takes longer than that then it's not functioning / not connected. Having 1ms as the minimum timeout could make it slower to detect a bad device.

But if you're calling this code from Python then 1ms would be ok as the minimum timeout. So maybe at the C level the function takes a timeout in microseconds, but at the Python level in milliseconds?

So maybe at the C level the function takes a timeout in microseconds, but at the Python level in milliseconds?

I'll leave this to you, but if there's already usecase for sub-ms timeout for this function, I'd say it should take us, regardless if it's Py or C.

but if there's already usecase for sub-ms timeout for this function, I'd say it should take us, regardless if it's Py or C

It's a timing function measuring microsecond pulses, so it makes sense to have the timout also measured in microseconds. I'll leave the timeout as us then.

Reworked and push in 4940bee, 93a9c2e and cff2b7a.

From my comment above please note:

Note also that if the timeout is too large (eg 10 seconds) and no pulse is detected, then the ESP will reset due to WDT timeout. It's not clear how to fix this while still retaining accuracy of timing the pulse (and also be portable to other ports).

Also, if we want to add the ability to wait for the starting edge of the pulse then that can be added later as per the discussion above using bit-flags to the pulse_level argument (in the docs I've specified this argument as needing to be either 0 or 1 to allow for a future enhancement).

Thanks! We definitely should consider this function "provisional" (in CPython terms) and expect that it's interface may be tweaked.